Align inference resolution with javac by applying hints from Maurizio#5004
Align inference resolution with javac by applying hints from Maurizio#5004stephan-herrmann wants to merge 6 commits into
Conversation
stephan-herrmann
force-pushed
the
ivar_ranking
branch
from
6ecda2b to
9ee5bfa
Compare
|
Once again builds fail in a way that's fully opaque to me: I didn't touch that test plugin, nor do I see any activity in that plugin other than the bump of parent versions in 1c8cd97.
I'm not inclined to investigate this build failure. |
|
May be you are impacted by #5013 (comment) too ? |
Thanks for the hint. So "my" build failure seems to be caused by eclipse-platform/eclipse.platform.releng.aggregator#3772 and would then need eclipse-platform/eclipse.platform.releng.aggregator#3784 |
|
@stephan-herrmann : please simply rebase your PR on master. |
stephan-herrmann
force-pushed
the
ivar_ranking
branch
from
3cc73e6 to
888c7cd
Compare
Done. I'd be surprised if |
Glad to surprise you :) |
|
OK, after build trouble got resolved, back to inference business: Despite the confirmation from #4937 (comment) I'm not quite ready to merge this, because test success depends on one tweak (from 22dc3e1) which I'm uneasy about. The tweak introduces use of SAME-bounds during the 2nd attempt of inference resolution and some 30 tests currently depend on it (look for code comment I came back to this tweak when addressing #4937: in 3cc73e6 I added code to |
|
Most of the tests needing the tweak from 22dc3e1 are test for record type inference. For a simple example I debugged RecordPatternTest.testIssue1336_1 record R<T> ( T t) {}
public final class X {
private static boolean foo(R<?> r) {
if (r instanceof R(String s)) {
return true;
}
return false;
}
}We start from innocent ivar dependencies (ivars corresponding to α1 and β1 in 18.5.5): Then resolution creates fresh type variables to yield (partly into incorporation): At this point we generate a new, failing constraint: We cannot equate two independent fresh type variables. Actually, resolving Since record pattern inference raises a few questions of its own, I tried to isolate matters, by letting only record pattern inference use the tweak in question. At this point only two tests fail due to disabling the tweak:
Both tests use enums in special ways. Is that an indication for the cause?? |
|
When the dust in this area settles I'll also revisit my NON-JLS invention in #5059 |
remove our own tweak from commit ad3653e
Improve the impl to cover one more issue + really make ivar-ivar dependencies bidirectional + also consider ivar-super bounds in ivar prioritization - but not so for same bounds Also fix an NPE in debug logging Fixes eclipse-jdt#4937
stephan-herrmann
force-pushed
the
ivar_ranking
branch
from
888c7cd to
7128ddc
Compare
+ only during regular inference: + record inverse of same-bounds for use by rankIVar() + only during record pattern inference: + workaround for inverse same-bounds in rankIVar() Still need to always use same-bounds during 2nd attempt of resolution Minor corrections (moving code lines up/down) Failures in + GenericsRegressionTest.test434118() + GenericsRegressionTest_9.testIssue4864()
Here are some fresh results from comparison with javac using latest from #4838: I used the test reduced to the method where ecj (with pending changes) and javac differ: public class X {
public Object convertFails(Class<?> clazz, String value) {
return Enum.valueOf(clazz.asSubclass(Enum.class), value);
}
}ecj and javac agree up until before final incorparation: javac: ecj: Notational differences:
Following this status behavior significantly differs! javac bluntly states: By contrast, ecj performs quite some incorporation work. In particular we have
At this point our bound set is This no longer resembles what javac tersely declares as "ditto". @srikanth-sankaran when debugging javac on the example in this comment, can you say more about this sequence:
Is there really no further activity from incorporation? |
- GenericsRegressionTest.test434118 - GenericsRegressionTest_9.testIssue4864 + more propagation of isSoft flag (for consistency) + slightly more debug output + make debug configurable per system property
|
Anyway, the above comparison hinted at where javac and ecj start to go separate paths, and even if information from javac after that point may be incomplete, I found a plausible tweak that actually fixes the two intermediate regressions: mark constraints derived from capture bounds as "soft". |
…k to false as recommended
Yes, the serialized textual representation of the boundsets at those points are identical. |
4 participants
Working towards replacing a speculative part of our fix from #4735 with "official" hints from https://mail.openjdk.org/archives/list/compiler-dev@openjdk.org/message/GN6RTCGMME6I5JVLSFZRIR32XY6QKOI2/
Our own tweak from ad3653e has been removed.
No new tests included because we only switch implementation strategies from our own invention to something closer to javac. For known test cases both strategies are indeed equivalent.